Add "\n" after title unconditionally in case the message is empty #34512
Conversation
If the message from form.MergeMessageField is empty, we will miss a "\n" between the title and the "Co-authored-by:" line. The title and message should have a blank line between of them.
GiteaBot
added
the
lgtm/need 2
routers/web/repo/pull.go
Outdated
| @@ -1109,8 +1109,9 @@ func MergePullRequest(ctx *context.Context) { | |||
| } | |||
|
|
|||
| form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) | |||
| message += "\n" | |||
There was a problem hiding this comment.
But if form.MergeMessageField was empty, then there will be a unnecessary \n used for the empty message?
There was a problem hiding this comment.
We encountered a case where the commit message is empty, and the code at this line appends "\nCo-authored-by: " + sig.String(), using only one \n instead of two. As a result, there is no extra blank line between the commit title and the message body.
Would it be acceptable to always add a \n after the commit title, even if the message body is completely empty (i.e., not even the auto-appended Co-authored-by: ... line)?
There was a problem hiding this comment.
Would it be acceptable to always add a \n after the commit title, even if the message body is completely empty (i.e., not even the auto-appended
Co-authored-by: ...line)?
Could you design some test cases to show the expected output for each case?
There was a problem hiding this comment.
Here is the case we encountered. The commit title and Co-authored-by: line don't have an extra blank line between of them if the commit message is empty. Then it looks like from git log command
and what we see from tig
I'm not familiar with gitea project. Can you guide me how can I add the testcase for my change? Thanks.
There was a problem hiding this comment.
I think we can do this: 669cccd
Then the \n can be handled correctly for all cases.
There was a problem hiding this comment.
Yeah. Check whether an extra \n' before Co-authored-by: line is another solution. But is a \n always added before the Co-authored-by: line? I think it would look like:
Reviewed-on: https://...
Reviewed-by: ...
Reviewed-by: ...
Co-authored-by: ...
Co-committed-by: ...
There was a problem hiding this comment.
Maybe it's better to have a function like addCommitMessageTailer("Co-authored-by", "..."), and let it handle all cases.
There was a problem hiding this comment.
Maybe it's better to have a function like
addCommitMessageTailer("Co-authored-by", "..."), and let it handle all cases.
Try this one: 26e1f78
If the message from form.MergeMessageField is empty, we will miss a "\n" between the title and the "Co-authored-by:" line. The title and message should have a blank line between of them.